Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Auto-apply variables #21

Open
wants to merge 34 commits into
base: master
Choose a base branch
from

Conversation

GoodOldJack12
Copy link
Contributor

@GoodOldJack12 GoodOldJack12 commented Oct 7, 2024

This PR adds a lot of modularity to the module template where it was previously hardcoded or on-boot only:

  • Option to automatically mount, create and grow filesystems of attached volumes
  • Automatically install/uninstall nginx when enabled/disabled
  • Automatically mount/unmount NFS and add/remove to fstab

It also adds more customization options:

  • Custom cron entries that are automatically added/removed
  • Support for custom cloud-init segments
  • Improved user-script support (no longer overrides default)

Finally it also fixes some issues when using the module as a proper external module.
Some of the features rely on ssh in order to run playbooks/install cron scripts.

Backwards-compatible in most cases but requires user-communication.

}
locals {
ansible_env={
ANSIBLE_REMOTE_PORT = var.host.port
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we still have a remote ansible? or is this always used locally?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes this is for local ansible to know what port to connect to

}
locals {
scripts_enabled = var.is_windows ? false : var.scripts_enabled
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add an extra new line at the end of the files, the same for the rest

ANSIBLE_REMOTE_USER = local.ssh_user
ANSIBLE_HOST_KEY_CHECKING = false
}
ansible_command="timeout 4m ansible-playbook -c ssh -i ${data.openstack_networking_floatingip_v2.public.address},"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do you use the public IP instead of the local _vm private one?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because otherwise users using terraform on their local laptop wont be able to connect to the vm

content = <<-EOF
#!/bin/bash
if [[ -r '/etc/debian_version' ]];then
apt-get update && apt-get install -y nfs-common cron
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I won't install nfs if the user/project does not want to use manila service


EOT
}
provisioner "remote-exec" {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not local-exec?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do have a Proof Of Concept for an ansible playbook. Only downside is that it doesn't really support the "time format string" like * * * * * and instead needs dedicated variables for minute/hour/day/weekday/...

provisioner "remote-exec" {
inline = [
"set -e",
"sudo cp /home/${local.ssh_user}/${random_id.obscure[each.key].id}-${each.key}.sh /opt/vsc/cron/${each.key}.sh",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are these files only accessible by root user?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The files are placed inside the user home directory first with default permissions, and are then immediately copied and chown-ed to root:

"sudo chown root:root /etc/cron.d/${each.key}",

There is a brief window where these files are not owned by root, but considering this user has access to sudo anyway that doesn't really make a difference. Nevertheless with an ansible solution we could place the files in the right directory immediately

@@ -3,14 +3,16 @@ locals {
any_enabled = var.nfs_enabled || var.nginx_enabled
ports = {
ssh = jsondecode(shell_script.port_ssh.output["ports"])[0]
http = var.nginx_enabled ? jsondecode(shell_script.port_http[0].output["ports"])[0] : null
http = var.nginx_enabled ? ( var.alt_http ? jsondecode(shell_script.port_http[0].output["ports"])[0] : 80 ) : null
https = var.nginx_enabled ? ( var.alt_http ? jsondecode(shell_script.port_http[0].output["ports"])[1] : 443) : null
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should add this in the current docs if we do not have it yet, ports 80 and 443 are open from ugent net

lifecycle {
precondition {
condition = var.public
error_message = ("Cant enable forward on a private instance!")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't

@alvarosimon
Copy link
Member

Nice! I have a few questions/comments @GoodOldJack12 . I guess this is backwards incompatible so we should prepare a email for the users before merging this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants